Skip to content

Conversation

@yannbf
Copy link
Member

@yannbf yannbf commented Jan 15, 2026

Closes #

What I did

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

Caution

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>

Summary by CodeRabbit

  • Tests
    • Updated test assertions to accept flexible numeric duration values in telemetry validations, improving test robustness without affecting application behavior.

Note: This release contains internal testing improvements with no user-facing changes.

✏️ Tip: You can customize this high-level summary in your review settings.

@yannbf yannbf self-assigned this Jan 15, 2026
@yannbf yannbf added build Internal-facing build tooling & test updates ci:normal labels Jan 15, 2026
@nx-cloud
Copy link

nx-cloud bot commented Jan 15, 2026

View your CI Pipeline Execution ↗ for commit a3a3a18

Command Status Duration Result
nx run-many -t compile,check,knip,test,pretty-d... ✅ Succeeded 6m 55s View ↗

☁️ Nx Cloud last updated this comment at 2026-01-15 11:41:52 UTC

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 15, 2026

📝 Walkthrough

Walkthrough

Test assertions updated to accept any numeric duration values instead of exact zeros in telemetry-related expectations across multiple test cases. No functional changes to control flow or underlying logic.

Changes

Cohort / File(s) Summary
Test Assertion Updates
code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
Changed candidateAnalysisDuration, totalRunDuration, and testRunDuration expectations from exact 0 values to expect.any(Number) for increased assertion flexibility in error and error-adjacent test scenarios.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f8d53b4 and a3a3a18.

📒 Files selected for processing (1)
  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Follow the spy mocking rules defined in .cursor/rules/spy-mocking.mdc for consistent mocking patterns with Vitest

Files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
**/*.test.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)

**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access the mocked functions in Vitest tests
Implement mock behaviors in beforeEach blocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking without vi.mocked() in Vitest tests
Avoid mock implementations outside of beforeEach blocks in Vitest tests
Avoid mocking without the spy: true option in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking with vi.mocked() in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests

Files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Format code using Prettier with yarn prettier --write <file>

Files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Run ESLint checks using yarn lint:js:cmd <file> or the full command cross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directives to fix linting errors before committing

Files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Enable TypeScript strict mode across all packages

Files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested, not just verify syntax patterns
Achieve high test coverage of business logic, aiming for 75%+ coverage of statements/lines
Cover all branches, conditions, edge cases, error paths, and different input variations in unit tests
Use vi.mock() to mock file system, loggers, and other external dependencies in tests

Files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use logger from storybook/internal/node-logger for server-side logging in Node.js code

Files:

  • code/core/src/core-server/server-channel/ghost-stories-channel.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: normal
  • GitHub Check: nx
  • GitHub Check: nx
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/core/src/core-server/server-channel/ghost-stories-channel.test.ts (3)

431-441: LGTM! Using flexible matchers for duration values is the correct approach.

Duration measurements are inherently non-deterministic even in error paths—execution time can vary across environments and runs. Switching to expect.any(Number) maintains type validation while eliminating timing-induced flakiness.


515-527: Consistent fix for the JSON report not found scenario.

The test correctly validates the telemetry payload structure while using flexible matchers for all duration fields. This aligns with the other changes and properly addresses timing-related flakiness.


568-580: LGTM! Completes the consistent fix across all error scenarios.

All three error condition test cases now use expect.any(Number) for duration fields, ensuring consistent behavior and eliminating flakiness while still validating that the telemetry payload contains the expected structure with numeric duration values.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Internal-facing build tooling & test updates ci:normal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants